Skip to content

refactor: relax objects allowed in model() #9493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
paulbalandan merged 1 commit into from
Mar 18, 2025
Merged

refactor: relax objects allowed in model() #9493

paulbalandan merged 1 commit into from
Mar 18, 2025

Conversation

@paulbalandan
Copy link
Member

@paulbalandan paulbalandan commented Mar 16, 2025

Description
I remember I have read somewhere in the codebase or in the documentation that model() can accept any object other than those extending CodeIgniter\Model as long as that class follow the semantics of the Model class. So, this relaxes the requirement for model() to accept only Model classes.

Supersedes and closes #9391 .

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@neznaika0
Copy link
Contributor

neznaika0 commented Mar 16, 2025

I looked at Factories - the model object is expected. Are you saying that you can return any object?

@paulbalandan
Copy link
Member Author

paulbalandan commented Mar 17, 2025

Yes, look at the PHPDoc of Fabricator::$model:

CodeIgniter4/system/Test/Fabricator.php

Lines 51 to 56 in 0611c35

/**
* Model instance (can be non-framework if it follows framework design)
*
* @var Model|object
*/
protected $model;

Looking at its constructor, it doesn't check for Model explicitly. Just plain object is fine.

CodeIgniter4/system/Test/Fabricator.php

Lines 122 to 131 in 0611c35

if (is_string($model)) {
// Create a new model instance
$model = model($model, false);
}
if (! is_object($model)) {
throw new InvalidArgumentException(lang('Fabricator.invalidModel'));
}
$this->model = $model;

Lastly, look at the return type of model():

CodeIgniter4/system/Common.php

Line 800 in 0611c35

* @phpstan-return ($name is class-string<ModelTemplate> ? ModelTemplate : object|null)

michalsn
michalsn approved these changes Mar 17, 2025
View reviewed changes
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPStan errors are unrelated?

@paulbalandan paulbalandan merged commit 5d1d53a into codeigniter4:develop Mar 18, 2025
6 checks passed
@paulbalandan paulbalandan deleted the model-argument-type branch March 18, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michalsn michalsn michalsn approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@paulbalandan @neznaika0 @michalsn